feat: scaffold transaction presentation in bdk_demo active wallet flow by j-kon · Pull Request #62 · bitcoindevkit/bdk-dart

j-kon · 2026-03-23T09:12:17Z

Summary

Adds a transaction detail flow to the reference wallet scaffold.

What changed

enabled navigation from transaction list to a transaction detail page
added TransactionDetailPage to display:
- full txid
- amount
- status
- block height
- timestamp
introduced WalletService.loadTransactionByTxid
made transaction rows tappable using Material/InkWell
refactored widget tests with FakeWalletService
added tests for:
- navigation to detail page
- correct transaction rendering
- handling missing transactions gracefully

Notes

uses placeholder transaction data
structured for future real wallet/BDK integration

reez · 2026-03-23T13:27:21Z

@Johnosezele cc

reez · 2026-03-24T15:23:50Z

Nice improvement to the demo flow and the widget coverage is solid.

I found one correctness issue before approval: TransactionDetailPage caches its future in initState, but go_router keys matched pages by the route pattern, so changing /transactions/:txid can reuse the same State and leave the screen showing the old transaction. Once that lifecycle case is handled, this is looking pretty good to merge.

j-kon · 2026-03-24T15:47:26Z

Nice improvement to the demo flow and the widget coverage is solid.

I found one correctness issue before approval: TransactionDetailPage caches its future in initState, but go_router keys matched pages by the route pattern, so changing /transactions/:txid can reuse the same State and leave the screen showing the old transaction. Once that lifecycle case is handled, this is looking pretty good to merge.

Good catch, that makes sense.

I’ll update the page to handle route changes properly instead of caching in initState.

j-kon · 2026-03-24T17:47:25Z

Good catch on that — makes sense.

I’ve fixed it by refreshing the cached future in didUpdateWidget whenever the txid changes, so the page no longer shows stale transaction data when the route is reused.

Also added a widget test to cover this exact case (same page instance, different txid).

Everything’s passing on my end. Happy to adjust if you prefer a different approach.

CC @reez

Johnosezele

Thanks for taking this on! Great changes in the UI side of things.

I recognize this as a scaffold pr but I think there's some mixup from the ground up

You're not fetching transaction details from CanonicalTx of bdk.dart where sent/received come from wallet.sentAndReceived().

For your changes on the wallet_service.dart file, you are mostly return hardcoded placeholder data, you're not making use of our bdk.dart bindings api, I can't see any of the related methods, can you take a look at this commit. By extension this has a huge impact on wallet_providers.dart

For the test section of your PR, We'd need to structure /bdk_demo/test because as our test suite grows things may begin to look tightly coupled, part of what I did in the /test section of this commit which is still draft.

@reez pls can you hold off on merging this just yet, there seem to be alot of parallel implementation conflict as this is a scaffold pr, it would need to be rebased due to lots of conflicting changes with [this](https://github.com/bitcoindevkit/bdk-dart/pull/61)

j-kon · 2026-03-25T00:54:01Z

Thanks a lot for the detailed review — this really helps.

I see what you mean about the mixup with using placeholder data instead of wiring through the bdk.dart APIs, and also the structure concerns around the demo/tests.

I’ll take a step back and align this properly with the existing architecture, rework the service layer to use CanonicalTx / wallet APIs, and clean up the structure as suggested.

Also noted on the README and shared widgets — I’ll split those changes out and refactor accordingly.

Appreciate the guidance here 🙏

reez · 2026-04-07T14:46:50Z

merged #61 so can rebase this branch

…tests

…ests

j-kon · 2026-04-14T18:43:40Z

merged #61 so can rebase this branch

Thanks, I’ve now rebased this branch.

reez · 2026-04-14T18:45:30Z

merged #61 so can rebase this branch

Thanks, I’ve now rebased this branch.

Cool I'll let John take a look to see if all his comments are resolved 👍

Johnosezele

I expect the Transaction flow to exist as a separate feature module, instead of being implemented by repurposing ActiveWalletsPage / wallet_setup to act like a “reference scaffold”.

I’d expect something closer to:

features/transactions/
- transactions_list_page.dart
- transaction_detail_page.dart
- transactions_controller.dart (Riverpod Notifier)
- transactions_repository.dart (interface)
services/ stays focused on actual wallet / persistence concerns (WalletService, StorageService, etc.)
any placeholder/demo data should live behind a demo TransactionsRepository (or fake) rather than inside WalletService.

Thanks for adding coverage, the widget tests are valuable (esp. the tx detail route reuse / txid refresh case).
That said, this PR still doesn’t really follow the earlier direction about keeping /bdk_demo/test structured as it grows (to avoid tightly-coupled test files and to keep helpers reusable).

I can see that FakeWalletService + placeholder transaction fixtures are defined inside app_shell_test.dart, and similar fixtures appear again in active_wallets_page_test.dart.

In the end, something like this is ideal:

test/presentation/transactions/transaction_detail_page_test.dart
test/presentation/wallet_setup/reference_wallet_scaffold_page_test.dart (or wherever this flow lands)
test/helpers/fakes/fake_wallet_service.dart
test/helpers/fixtures/placeholder_transactions.dart

This way tests remain feature-scoped, helpers are shared, and future PRs can add coverage without growing single files indefinitely.

j-kon · 2026-04-19T08:26:38Z

Thanks @Johnosezele, this is clear.

I see the core issue now: the transaction scaffold should be introduced as its own feature module instead of being layered onto ActiveWalletsPage and WalletService.

I’m going to refactor this in that direction by:

restoring ActiveWalletsPage to its original behavior
moving the scaffold UI into a standalone transactions feature
keeping demo transaction data behind a demo repository
restoring WalletService to a strict contract
renaming the app-side transaction model to avoid the hide TxDetails pattern
removing balanceDelta so net amount is always derived from received - sent
splitting the tests so active-wallet coverage stays intact and transaction tests move out of app_shell_test.dart

Thanks for the detailed review.

Johnosezele · 2026-04-19T20:38:34Z

Maybe in the future try to create a commit per fix, its easier to review that way. I can see all of my comments marked as resolved but its hard to track which is which from the diff in your recent commit.
Thanks again.

j-kon · 2026-04-19T22:15:48Z

Thanks.

I agree the last refactor landed as a larger commit than ideal. I’ll keep future follow-up work split into smaller fix-by-fix commits so it’s easier to review and map comments back to changes.

Appreciate the review.

Johnosezele suggested changes Mar 24, 2026

View reviewed changes

j-kon added 3 commits April 14, 2026 13:58

feat: scaffold transaction presentation in bdk_demo active wallet flow

0e9986f

feat: add transaction detail flow with navigation and tests

c6fcca9

feat: enhance transaction detail page to refresh on txid change with …

bebb956

…tests

j-kon force-pushed the feat/bdk-demo-transaction-scaffold branch from 2d3e933 to bebb956 Compare April 14, 2026 13:00

j-kon added 2 commits April 14, 2026 14:26

feat: refine wallet service imports and enhance active wallets page t…

49ee27a

…ests

fix: reformat wallet_service.dart for consistency

8c77d9d

j-kon added 2 commits April 15, 2026 02:38

chore: drop bdk_demo README changes from transaction scaffold PR

380d1e0

refactor: share wallet UI helpers in bdk_demo

d46ca6e

Johnosezele suggested changes Apr 17, 2026

View reviewed changes

refactor: split bdk_demo transactions into standalone feature

5b46853

Conversation

j-kon commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Notes

Uh oh!

reez commented Mar 23, 2026

Uh oh!

reez commented Mar 24, 2026

Uh oh!

j-kon commented Mar 24, 2026

Uh oh!

j-kon commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Johnosezele left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

j-kon commented Mar 25, 2026

Uh oh!

reez commented Apr 7, 2026

Uh oh!

j-kon commented Apr 14, 2026

Uh oh!

reez commented Apr 14, 2026

Uh oh!

Johnosezele left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

j-kon commented Apr 19, 2026

Uh oh!

Johnosezele commented Apr 19, 2026

Uh oh!

j-kon commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

j-kon commented Mar 23, 2026 •

edited

Loading

j-kon commented Mar 24, 2026 •

edited

Loading

j-kon commented Apr 19, 2026 •

edited

Loading